-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: type definitions, es modules usage #25
Conversation
Signed-off-by: Tom Richards <[email protected]>
Signed-off-by: Tom Richards <[email protected]>
@t-richards thanks for the PR, but I would like you to open an issue first to discuss what is the problem first and what's the best way to solve it. I don't see what the problem is right now? |
Comment moved --> #26. |
@t-richards yes please open an issue with your comment and let's continue the discussion there. |
cometd-nodejs-client.d.ts
Outdated
@@ -0,0 +1,12 @@ | |||
export interface Proxy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add the license boilerplate at the beginning of the file - copy it from cometd-nodejs-client.js
.
cometd-nodejs-client.d.ts
Outdated
@@ -0,0 +1,12 @@ | |||
export interface Proxy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to rename this to HttpProxy
?
@t-richards, since you've done it, would you contribute also (maybe another PR) at least one test (converted or new) in TS? Can tests be run by |
Signed-off-by: Tom Richards <[email protected]>
@sbordet I believe that That said, if the test conversion process from JS -> TS is somewhat straightforward, I think it might make sense to convert them all in one go and not bother with the mixed mode. I will try this out in a separate PR. |
This PR:
.d.ts
) to the library.Testing
How I tested these type definitions locally:
test/
files from.js
->.ts
@types/node
,@types/mocha
,ts-node
,typescript
packagesconst
->let
mocha -r ts-node/register test/*.ts